Use patched version of zlib-ng to fix install name #8743
Conversation
radarhere
changed the title
|
You may or may not have realised that the Windows build has seven "patches" of its own (they're not true patches at the moment, but rather find and replace) - https://github.com/python-pillow/Pillow/blob/main/winbuild/build_prepare.py#L139 Do you think these should be moved to be part of a "patches" directory as well? |
|
I wasn't aware of the Windows patching strategy - I haven't really spent any time looking into the Windows build, since there's no overlap with iOS or macOS. I agree it looks like those patches would be at least a candidate for migrating to a common |
|
I'm not a huge fan of including patch files in repos, I have memories of diffing patch files after an upstream has shifted, which wasn't much fun, so I'm leaning towards #8673. @radarhere But if you have a stronger preference, please go ahead and merge any of these three to enable testing (re: #8672 (comment)). And the good news is zlib-ng/zlib-ng#1867 has been approved, so hopefully that will be merged and released at some point. |
|
#8673 has been merged. |
|
@hugovk Hrm... that's going to get interesting when the iOS patch lands, because some of the dependencies require light patching (mostly to the build systems). However, I guess we can cross that bridge when we get to it :-) (Edit: By "lands", I mean "lands in the PR queue for review" - I'm not presuming it will be merged unless it passes review) |
|
Yeah, let's see. I'm not totally against it, and let's do whatever's practical to cross the bridge. |
Fixes #8671. Alternative to #8672 and #8673
When zlib-ng is compiled on macOS, it sets the install name of the libz.1.dylib to @rpath/libz.1.dylib. This means that any subsequent load requires a valid DYLD_LIBRARY_PATH to resolve the link.
Recent versions of macOS have a feature called System Integrity Protection (SIP) which (amongst other things) prevents DYLD_LIBRARY_PATH from being passed into subshells. As the build's dependencies aren't on the default library path on macOS, delocate-wheel is unable to resolve the libz library.
SIP is disabled on GitHub Actions configurations, so this problem isn't seen in CI; but is enabled by default on macOS machines, so individual developers building Pillow dependencies will get an error from cibuildwheel when the wheel is repaired.
This PR uses the
PATCH_DIRmechanism provided by multibuild to provide a patch forzlib-ng(authored by @radarhere, submitted upstream as zlib-ng/zlib-ng#1867) to fix the issue with zlib-ng's install dir during the original build.This is an alternative to:
DYLD_LIBRARY_PATHsodelocate-wheelis able to resolve@rpath; andinstall_name_toolto fix the dylib after it is compiled.I'd argue this is the best of the three approaches, as it fixes the root problem at the source, in a way that will (eventually) require no patch at all.